sioyek: fix typo in darwin build steps #348045
Conversation
NixOSInfra
added
the
12. first-time contribution
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
10.rebuild-linux: 0
There was a problem hiding this comment.
Wow, yikes, sorry I didn't catch this in review. Looks like it happened in #283813 (comment) and none of us caught it since we don't build on macOS.
Do you happen to know why the build is failing on OfBorg? No worries if not.
|
@xyven1 only tangentially related to this PR, but do you know what is going on in the linux nixpkgs/pkgs/by-name/si/sioyek/package.nix Lines 68 to 71 in 9ed5dad If I check with nixpkgs/pkgs/by-name/si/sioyek/package.nix Lines 51 to 53 in 9ed5dad with substituteInPlace pdf_viewer/main.cpp \
--replace-fail "/usr/share/sioyek" "$out/share/siyoek" \
--replace-fail "/etc/sioyek" "$out/etc/sioyek"and remove lines 68--70. Having them outside clutters my nix profile ( Also we are missing |
b-fein
force-pushed
the
sioyek-fix-darwin-build
branch
2 times, most recently
from
07d25ff to
70a3675
Compare
b-fein
force-pushed
the
sioyek-fix-darwin-build
branch
from
70a3675 to
ad80cc3
Compare
It looks like |
wegank
added
12.approvals: 2
|
Finished from my side, as far as I’m concerned it is ready to be merged. |
|
Hi, please check ofborg failure (on darwin) below |
|
Result of 1 package failed to build:
Failing with: Currently at NixCon I might have time later to have a closer l👀k… |
|
any update? |
|
This fails for me with (after a rebase): Though it worked before the latest SDK updates. |
|
|
The immediate failure comes from the fact that the QtMultimedia framework is passed as: I think this I have no idea why this happens though. Someone with some knowledge around |
|
This is probably my fault, I messed with the Darwin Qt 6 build recently. Sadly I don’t actually know that much about Qt :) but I can try and take a look. If you could try with #352395 reverted it would be helpful. |
|
Oh good, I’m off the hook then :) Did it build before the Qt 6.8 update? It’d be good to see the |
b-fein
force-pushed
the
sioyek-fix-darwin-build
branch
from
ad80cc3 to
ba0f5ad
Compare
b-fein
force-pushed
the
sioyek-fix-darwin-build
branch
from
ba0f5ad to
e001379
Compare
b-fein
changed the title
|
Since I have little experience with QT and definitely none whatsoever with how Nixpkgs does QT-library-stuff internally, I do not think this PR is the right place to debug this further. With this: can we merge this PR? (If I can help to test a Darwin build of sioyek in the future, feel free to ping me.) |
|
Apologies for not being of more help, can't test on MacOS. As a workaround for now, is it possible to revert the changes in #283813 with an overlay like the following? final: prev:
{
sioyek = prev.sioyek.overrideAttrs (previousAttrs: {
src = final.fetchFromGitHub {
owner = "ahrm";
repo = "sioyek";
rev = "v2.0.0";
sha256 = "sha256-GFZaTXJhoBB+rSe7Qk6H6FZJVXr3nO9XgM+LAbS4te4=";
};
buildInputs = builtins.filter (pkg: final.lib.getName pkg != "qtspeech")
previousAttrs.buildInputs or [ ];
});
}If I remember correctly, the changes to the build were fairly minimal. If there are problems due to the final: prev:
let
sioyek' = prev.sioyek.overrideAttrs (previousAttrs: {
src = final.fetchFromGitHub {
owner = "ahrm";
repo = "sioyek";
rev = "v2.0.0";
sha256 = "sha256-GFZaTXJhoBB+rSe7Qk6H6FZJVXr3nO9XgM+LAbS4te4=";
};
buildInputs = builtins.filter (pkg: final.lib.getName pkg != "qtspeech")
previousAttrs.buildInputs or [ ];
});
in
{
sioyek = final.qt5.callPackage sioyek'.override { };
}If you're up for it, I would review a change that adds a separate Darwin derivation like what we do for signal-desktop.
Neither I nor xyven1 (I think) have merge permissions, so you can play the waiting game or ping one of the participants here who do. Or you can post on one of the NixOS discourse PR review threads like I did in #283813 (review). |
wegank
removed
12.approvals: 2
After #283813, the
postInstallstep on Darwin tries to copy a file that does not exist.Fixes the typo in the file name so that copying works.
Relevant
pdf_viewer/keys_user.configfile: https://github.com/ahrm/sioyek/blob/development/pdf_viewer/keys_user.configThings done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.